Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

5137.3 Requisition line edit + UI updates to line columns #5308

Merged
merged 37 commits into from
Nov 7, 2024
Merged

Conversation

roxy-dao
Copy link
Contributor

@roxy-dao roxy-dao commented Nov 4, 2024

Part 3 of #5137

πŸ‘©πŸ»β€πŸ’» What does this PR do?

  • Allows the users to add lines to the response requisition if it is a general manual requisition.
  • Adds new ListOption component that lists an array of options
  • Create new response requisition line page which:
    • Lists the lines for the response requisition the left side of the page
    • Allows users to edit the columns in the right side of the page
  • The user can select the previous/next buttons on the page to go to the previous or next buttons OR click on the item in the list to navigate to that line
  • The tick will toggle on if the user has entered suggested quantity
  • Refactored insert response line to only accept id, item_id and requisition_id
Screen.Recording.2024-11-05.at.12.03.28.PM.mov

πŸ’Œ Any notes for the reviewer?

P.S. I forgot about the charts and only just remembered while writing up this PR so there will be a part 4 incoming! πŸ˜“
Carryover: Don't let user navigate away from page if reason isn't inputted and is required?

πŸ§ͺ Testing

  • Create general response requisition
  • Try add lines
  • Change item through the list / previous / next buttons
  • Input values
  • Make sure they save

πŸ“ƒ Documentation

  • Part of an epic: documentation will be completed for the feature as a whole
  • No documentation required: no user facing changes or a bug fix which isn't a change in behaviour
  • These areas should be updated or checked:
    1.
    2.

@github-actions github-actions bot added the enhancement New feature or request label Nov 4, 2024
Copy link

github-actions bot commented Nov 4, 2024

Bundle size difference

Comparing this PR to main

Old size New size Diff
4.95 MB 4.95 MB -5.28 KB (-0.10%)

@@ -145,7 +145,7 @@ export const UpdateStatusButtonComponent = ({
nextButton={
currentTab === Tabs.Status ? (
<DialogButton
variant="next"
variant="next-and-ok"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming of the next button so I can create an actual 'next' button not 'ok & next'

Comment on lines -2937 to -2976
additionInUnits?: InputMaybe<Scalars['Float']['input']>;
averageMonthlyConsumption?: InputMaybe<Scalars['Float']['input']>;
comment?: InputMaybe<Scalars['String']['input']>;
daysOutOfStock?: InputMaybe<Scalars['Float']['input']>;
expiringUnits?: InputMaybe<Scalars['Float']['input']>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need all these fields on insert. We will do everything on update as everything is saved on blur except for the actual line creation itself

@@ -6469,6 +6457,7 @@ export type RequisitionLineNode = {
remainingQuantityToSupply: Scalars['Float']['output'];
/** Quantity requested */
requestedQuantity: Scalars['Float']['output'];
requisitionNumber: Scalars['Int']['output'];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed for navigation

Comment on lines +30 to +47
const startIcon = (
<CheckIcon
style={{
backgroundColor: '#33A901',
borderRadius: '50%',
padding: '2px',
color: 'white',
height: 18,
width: 18,
}}
/>
);

const endIcon = (
<ChevronDownIcon
style={{ width: 17, height: 17, transform: 'rotate(-90deg)' }}
/>
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had them passed in as props at first since we might use different icons for different use cases, but can't think of them right now for what we will use it for. Easy to refactor later on if needed

Comment on lines +46 to +49
<DialogButton
variant="previous"
disabled={!hasPrevious}
onClick={() =>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using existing buttons for now instead of re-designing to fit new UI designs

Comment on lines +40 to +42
const lines =
data?.lines.nodes.sort((a, b) => a.item.name.localeCompare(b.item.name)) ??
[];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorting so the list is in alphabetical order

Comment on lines +20 to +21
export const useDraftRequisitionLine = (item?: ItemRowFragment | null) => {
const { id: reqId, lines } = useResponse.document.fields(['id', 'lines']);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using existing pattern for now, but this could be updated straight from endpoint instead of going through a draft

@roxy-dao roxy-dao changed the title 5137 Requisition line edit + UI updates to line columns 5137.3 Requisition line edit + UI updates to line columns Nov 4, 2024
@roxy-dao roxy-dao requested a review from Chris-Petty November 4, 2024 23:24
Base automatically changed from 5137.2-Requisition-line-edit-+-UI-updates-to-line-columns to develop November 5, 2024 22:24
An error occurred while trying to automatically change base from 5137.2-Requisition-line-edit-+-UI-updates-to-line-columns to develop November 5, 2024 22:24
@Chris-Petty Chris-Petty self-assigned this Nov 6, 2024
Copy link
Contributor

@Chris-Petty Chris-Petty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some wee nit and comments but otherwise works great!

"button.next-step": "Next step",
"button.ok": "OK",
"button.ok-and-next": "OK & Next",
"button.open-the-menu": "Open the menu",
"button.order-more": "Order more",
"button.previous": "Previous",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than this and Next the rest of this is just your editor alphabetically sorting aye?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct~

client/packages/common/src/intl/locales/en/common.json Outdated Show resolved Hide resolved
client/packages/common/src/types/schema.ts Outdated Show resolved Hide resolved
/>
}
labelWidth={LABEL_WIDTH}
label={t('label.outgoing')}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess it's just up the design but I'd have assumed we should have the header as "Outgoing Units". Maybe it's redundant given everything is in units though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sick looks great!

startIcon
) : (
<Box style={{ visibility: 'hidden' }}>{startIcon}</Box>
)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is quite funky - behind the scenes it appears to still be rendering the icon it's just invisible. But if you remove the icon JSX from here it renders the same anyway, just shadow dom doing less work?

)}
<Box
style={{
visibility: enteredLineIds?.includes(option.id)
Copy link
Contributor

@Chris-Petty Chris-Petty Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is in response to my comment I was more meaning that for this requisition of 21 items

image

image

It renders the checkbox component 63 times (3 times per row for whatever reason 😁) when only one row is actually showing the checkbox. Where as if you had

              <Box>{enteredLineIds?.includes(option.id) && startIcon}</Box>

Instead it only renders a checkbox 3 times, for the one row it's actually displayed on. Same goes for the endIcon

Copy link
Contributor Author

@roxy-dao roxy-dao Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no it was just a refactor.

I have both icons hidden because I didn't want it to move the text when it appears... which happens... So, instead we render it in a hidden state so the text doesn't move

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right I had to fiddle around a bit to see only seems to happen on a small window, so probably would happen on tablet!

@roxy-dao roxy-dao merged commit 062328a into develop Nov 7, 2024
6 checks passed
@roxy-dao roxy-dao deleted the 5137.3 branch November 7, 2024 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants